[PATCH] client: prohibit unprivileged users from setting sgid/suid bits

Prior to fb1b72d, unprivileged users could add mode bits as long as
S_ISUID and S_ISGID were not included in the change.

After fb1b72d, unprivileged users were allowed to modify S_ISUID and
S_ISGID bits only when no other mode bits were changed in the same
operation. This inadvertently permitted unprivileged users to set
S_ISUID and/or S_ISGID bits when they were the sole bits being modified.

This behavior should not be allowed. Unprivileged users should be
prohibited from setting S_ISUID and/or S_ISGID bits under any
circumstances.

This change tightens the permission check to prevent unprivileged
users from setting these privileged bits in all cases.

Signed-off-by: Kefu Chai <tchaikov@gmail.com>
origin: backport, https://github.com/ceph/ceph/commit/7028ed21138522495df1e9f8b01195a3c43d47ff
bug-debian: https://bugs.debian.org/1109470
bug: https://github.com/ceph/ceph/pull/64356

Gbp-Pq: Name CVE-2025-52555-2.patch

diff --git a/src/client/Client.cc b/src/client/Client.cc
index a4eb5054725c09507fe239075e2ca50277db2bf2..7153f1c1f4688260ba27f644500c92b021f5b39e 100755 (executable)
--- a/src/client/Client.cc
+++ b/src/client/Client.cc
@@ -5448,22 +5448,23 @@ int Client::may_setattr(Inode *in, struct ceph_statx *stx, int mask,
   }
 
   if (mask & CEPH_SETATTR_MODE) {
-    bool allowed = false;
     /*
      * Currently the kernel fuse and libfuse code is buggy and
      * won't pass the ATTR_KILL_SUID/ATTR_KILL_SGID to ceph-fuse.
      * But will just set the ATTR_MODE and at the same time by
      * clearing the suid/sgid bits.
      *
-     * Only allow unprivileged users to clear S_ISUID and S_ISUID.
+     * Only allow unprivileged users to clear S_ISUID and S_ISGID.
      */
-    if ((in->mode & (S_ISUID | S_ISGID)) != (stx->stx_mode & (S_ISUID | S_ISGID)) &&
-        (in->mode & ~(S_ISUID | S_ISGID)) == (stx->stx_mode & ~(S_ISUID | S_ISGID))) {
-      allowed = true;
-    }
-    uint32_t m = ~stx->stx_mode & in->mode; // mode bits removed
-    ldout(cct, 20) << __func__ << " " << *in << " = " << hex << m << dec <<  dendl;
-    if (perms.uid() != 0 && perms.uid() != in->uid && !allowed)
+    uint32_t removed_bits = ~stx->stx_mode & in->mode;
+    uint32_t added_bits = ~in->mode & stx->stx_mode;
+    bool clearing_suid_sgid = (
+      // no new bits added
+      added_bits == 0 &&
+      // only suid/suid bits removed
+      (removed_bits & ~(S_ISUID | S_ISGID)) == 0);
+    ldout(cct, 20) << __func__ << " " << *in << " = " << hex << removed_bits << dec <<  dendl;
+    if (perms.uid() != 0 && perms.uid() != in->uid && !clearing_suid_sgid)
       goto out;
 
     gid_t i_gid = (mask & CEPH_SETATTR_GID) ? stx->stx_gid : in->gid;